Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure we're loading .env in UTF-8 #1965

Closed
wants to merge 3 commits into from
Closed

Ensure we're loading .env in UTF-8 #1965

wants to merge 3 commits into from

Conversation

uranusjr
Copy link
Member

Since python-dotenv does not provide this capacity, we do the reading part ourselves instead (into a StringIO). It supports this at least, fortunately.

Fix #1963.

Since python-dotenv does not provide this capacity, we do the reading
part ourselves instead (into a StringIO). It supports this at least,
fortunately.
@uranusjr
Copy link
Member Author

I don’t get the Mercury failure…

@@ -0,0 +1,29 @@
try:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this really need its own file

single function => probably should stay in the main file to avoid the overhead

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It used to contain a subclass of StringIO to abstract away some text stream logic between Python 2 and 3, but I ended up just reading the whole thing out (since the dotenv file would be reasonably small anyway). Also I wanted to trim the 2000-line monster core.py as much as possible… but no, this doesn’t need its own file.

@uranusjr uranusjr closed this Apr 13, 2018
@uranusjr
Copy link
Member Author

Note to self: Also need to fix do_shell and cli.shell.

@techalchemy techalchemy deleted the dotenv-encoding branch September 5, 2018 05:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants